-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
7903928: update build.gradle to use JDK 23 #271
base: master
Are you sure you want to change the base?
Conversation
👋 Welcome back sundar! A progress list of the required criteria for merging this PR into |
@sundararajana This change now passes all automated pre-integration checks. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
Webrevs
|
changed Python sample so that it does not fail with unsupported type _Float16
@@ -29,6 +29,7 @@ def static checkPath(String p) { | |||
} | |||
|
|||
def llvm_home = project.property("llvm_home") | |||
def jdk_home = project.property("jdk23_home") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should rename this property simply to jdk_home
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. Especially now that we only have one "master" (tip) version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gradle's jdk is set via JAVA_HOME. It is usual have gradle support for the latest JDK to come bit "late". If we call JDK needed for jextract to be "jdk_Home" and the one needed for gradle as "JAVA_HOME", we may be in a situation where jdk_home points latest JDK (say JDK 25) and JAVA_HOME points whatever then latest Gradle supports. Can we keep jextract use specific JDK version for its build/test as a separately distinctly named variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can run gradle's Java compilation and the gradle tool with different JDKs; my previous strategy for running JMH through gradle was to define my local openjdk build as org.gradle.java.home
project property, either set in gradle.properties or passed as -Porg.gradle.java.home=xx
on command line. JAVA_HOME
is used to launch the gradle tool, so it cannot be the local openjdk build, which uses a class file format too new for gradle to generate abstract method implementations.
The |
This PR has been superseded by #275. Please continue the discussion and review there. |
We've updated native makefile to use JDK 23 (
0b89a46 )
While build.gradle works & expects jdk 23, it still refers to JDK var as "jdk22_home" and passes --release to be 22. Also jextract version is "22".
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jextract.git pull/271/head:pull/271
$ git checkout pull/271
Update a local copy of the PR:
$ git checkout pull/271
$ git pull https://git.openjdk.org/jextract.git pull/271/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 271
View PR using the GUI difftool:
$ git pr show -t 271
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jextract/pull/271.diff
Using Webrev
Link to Webrev Comment